Skip to content

feat(data-modeling): add Lucid models, migrations and seeders for the EER schema - #2

Merged
EdgarEldy merged 44 commits into
developfrom
feature/data-modeling
Aug 24, 2026
Merged

EdgarEldy merged 44 commits into
developfrom
feature/data-modeling

Conversation

@EdgarEldy

Copy link
Copy Markdown
Owner

Summary

This branch introduces every Lucid model, migration and seeder needed to run the core and auth schema described in the README's Data Model and EER_AUTH sections. The default database connection moves from the scaffold's SQLite to PostgreSQL, since that is the only connection this project targets from here on, and the auth wiring inherited from the initial kit=api scaffold (a tokens guard, a demo signup and login flow, a users table shaped around a single fullName column) is removed in favor of a plain User model matching the EER_AUTH design, since the built in access tokens provider has no place in a project that authenticates through a stateless JWT guard built later in feature/auth.

Ten models now exist under app/models, covering Category, Product, Customer and Order for the core schema and User, Role, Permission, ActivationToken, BlacklistedToken and PasswordResetToken for the auth schema, each with the relations the README specifies. Two migrations create all twelve tables, verified against a live PostgreSQL instance to run and roll back cleanly and reproduce the same schema on a second run. Five seeders populate realistic sample data and two ready to use accounts, an ADMIN and a USER, with credentials read from the typed env service so later feature branches can authenticate in functional tests without depending on feature/auth being merged first.

Task checklist

Every non test task in the README's feature/data-modeling checklist is complete: the pg driver is installed and configured as the default connection, all ten models exist with the relations and hooks described, both migrations are written and verified end to end against Postgres, all five seeders exist and were verified to produce correctly hashed passwords and correct role assignments, the seeded test credentials are exposed through .env.test and the typed env schema, and no synchronize usage exists anywhere in the codebase. All four test tasks are covered under tests/unit: the Category hasMany relation, the Order total round trip, the User beforeSave hashing behavior including the no-op case when password is unchanged, and a migration run and rollback check.

Test plan

npm run typecheck, npm run lint and npm test all pass locally against a live PostgreSQL test database, ten tests passing with zero failures. Migrations were run and rolled back directly against Postgres outside of the test suite as well, to confirm the up and down methods are symmetric and that the seeders are idempotent under the default node ace db:seed invocation, which runs every seeder file in the directory including MainSeeder's own orchestration.

The data modeling branch commits to PostgreSQL as the only supported
database connection, matching the tech stack table in the README and
the docker-compose services already defined for development and test.
The scaffold generated by the initial --kit=api install only shipped
better-sqlite3, so the pg driver needed to be added before Lucid could
open a connection against the docker-compose PostgreSQL service.

No other dependency changed. The lockfile update reflects only the
resolution of pg and its own transitive dependencies.
The scaffold from the initial --kit=api install defaulted to SQLite
with Lucid's schema generation feature enabled, which is convenient
for a throwaway prototype but not what this tutorial is built around.
The README fixes PostgreSQL 16 as the database for every environment,
run through docker-compose in development and docker-compose.test in
CI, so the default connection needs to point there from the moment
migrations start being written.

The pg connection now reads its credentials from the typed env
service instead of hardcoded values, consistent with the project rule
against touching process.env directly. SQLite is kept as a commented
alternative for reference, following the same pattern already used for
the mysql, mssql and libsql connections in this file, rather than
deleting it outright.
The --kit=api scaffold wired a tokens guard on top of a User model
generated by AdonisJS itself, which is not the User model this project
needs. The EER_AUTH design in the README calls for a User with roles
and permissions instead of a self-managed access tokens table, and
authentication in this project is meant to be handled by a stateless
JWT guard built in feature/auth, not the built-in tokens provider.

Keeping the tokens guard around while reshaping the User model in this
branch would leave config/auth.ts referencing a static accessTokens
provider that the new model does not implement, which breaks the
build. The session guard is left in place on its own as the default so
the auth module still exports a valid configuration and the app keeps
booting; it is a temporary placeholder until feature/auth replaces it
with the real JWT guard.
This controller was generated by the --kit=api starter and creates a
user from a fullName, email and password payload against the model
that ships with that starter kit. Neither the payload shape nor the
underlying User model match the EER_AUTH design this project follows,
where a user has separate first and last names plus roles instead of a
single fullName field and a bundled access token.

Registration for this project is a feature/auth concern: it needs an
activation token flow, a default USER role assignment and a password
policy that this scaffold controller never implemented. Removing it
now avoids carrying dead code that would not typecheck once the User
model changes in this branch, rather than patching it toward a shape
it was never designed for.
This controller implemented login and logout against the accessTokens
provider that the --kit=api starter attaches to its default User
model. That provider no longer exists once the User model is rebuilt
around the EER_AUTH schema, and the login flow it implemented does not
match what this project needs anyway: password verification through
hash.verify, an enabled and accountLocked check, and a JWT issued with
a jti claim used later for blacklisting on logout.

Rather than adapt this controller piecemeal toward a design it was
never written for, it is removed here and left for feature/auth, which
owns the AuthService and AuthController tasks in the README and has
the full picture of the login and logout contract to implement them
correctly in one pass.
The profile controller only serialized the authenticated user through
UserTransformer, which read fields such as fullName and an initials
getter that belonged to the --kit=api starter model. None of those
fields exist on the User model this branch introduces, so the
transformer and this controller would no longer compile once the
model changes.

The equivalent endpoint, GET /api/v1/auth/me, is defined in the README
as part of feature/auth and returns the user with roles and
permissions preloaded, which is a different contract than a bare
serialized user object. Removing the placeholder here keeps the branch
free of dead code rather than adapting a controller that feature/auth
would replace outright.
signupValidator and loginValidator were written against the --kit=api
demo model, validating a fullName field and a passwordConfirmation
that the README's auth flow does not use. The EER_AUTH design splits
the name into firstName and lastName and drives registration through
an AuthService that also creates an activation token, which this
validator has no concept of.

The real registerSchema and loginSchema belong to feature/auth, listed
explicitly in its task checklist alongside the password complexity
rules the README specifies. Deleting this file now avoids leaving a
validator that nothing imports anymore once the demo controllers that
used it are gone.
UserTransformer picked fullName, email, createdAt, updatedAt and an
initials getter off the demo User model to shape API responses. All of
those fields either no longer exist or no longer make sense once the
User model is rebuilt around firstName, lastName, enabled and
accountLocked, and password exposure is already handled at the model
level through serializeUsing rather than through a separate
transformer class.

This project's response shaping goes through the ApiResponse envelope
and the respond helper defined in feature/core-architecture, not
through per-model transformer classes, so this file has no equivalent
to be rebuilt toward. It is removed along with the controllers that
were its only callers.
This migration created a users table with full_name, email and
password columns, matching the demo User model from the --kit=api
starter. It conflicts directly with the users table this branch
defines in the EER_AUTH schema, which adds first_name, last_name,
enabled and account_locked and is created together with roles,
permissions and the pivot and token tables in a single migration.

Keeping both migrations would try to create the same table twice with
incompatible columns. Since this project has not been deployed
anywhere yet, there is no production history to preserve, so removing
the old migration outright is safe and avoids adding a second
migration just to alter columns.
The auth_access_tokens table backed the tokens guard from the
--kit=api scaffold, storing a hash and abilities per token against a
tokenable_id foreign key. This project does not use that provider:
authentication issues stateless JWTs and tracks revocation through a
separate blacklisted_tokens table keyed by jti, which is a fixed-size
claim rather than a hashed token value, and does not need a
tokenable_id since a JWT already carries the user's identity in its
payload.

Removing this migration avoids creating a table that nothing in the
EER_AUTH design would ever read from or write to. The blacklisted
tokens table is created instead as part of the auth schema migration
in this branch.
This file is generated automatically by Lucid's schema generation
feature and is not meant to be hand edited, its own header says so.
It reflected the auth_access_tokens and users tables from the SQLite
scaffold, both of which are gone now that the default connection is
PostgreSQL and the auth schema migration defines a different set of
columns and tables.

Schema generation was only wired up for the SQLite connection, which
is now commented out in config/database.ts, so there is no active
generator that would refresh this file against the new schema. Leaving
a stale, auto-generated file in the tree is worse than removing it: it
would read as documentation of a schema the project no longer has.
The auth and account groups routed to the three demo controllers this
branch removes, none of which match the endpoints the README defines
for this project. Leaving the group in place with a dangling import of
the generated controllers registry would fail to compile the moment
those controller files disappear.

The health check route is untouched since it does not depend on the
User model or any auth guard. Resource routes for auth, categories,
products, customers and orders are added incrementally by their own
feature branches once the controllers and services behind them exist,
starting with feature/auth.
This registry is generated automatically at build time from the
controllers present in app/controllers, and its own header warns
against editing it by hand. Once the three demo auth controllers were
removed, the checked-in snapshot still referenced them, which would
fail to resolve during typecheck since the underlying files no longer
exist.

Running node ace codegen regenerated both this file and the associated
data type declarations to match the current controller set, which at
this point in the project is just the health check. Later branches
will regenerate this file again as they add their own controllers.
The catalog hierarchy starts with categories, so this is the first model on
the branch. It only declares the two columns the README's Column Details
table lists for this table, id and category_name, rather than following the
generic timestamped example from the patterns skill. The skill's Category
example includes created_at and updated_at, but the README's own schema
table for this project omits them entirely for categories, products,
customers and orders, so the migration that will back this model does not
create those columns and the model should not declare properties that do
not exist in the database.

The hasMany relation to Product is declared with a lazy () => Product
closure, which is what lets category.ts and product.ts import each other at
the top of the file without deadlocking on circular ES module resolution.
The closure is only invoked when the relation is actually traversed at
runtime, long after both modules have finished evaluating.
Product is the many side of the categories to products relation and the one
side of products to orders. Its columns match the README's Column Details
table exactly: categoryId as the foreign key, productName and unitPrice,
with no timestamp columns since none are listed for this table.

unitPrice is typed as a plain number here even though the README's
constraint column says "> 0". That constraint is enforced at the database
level in the migration with a check constraint rather than in the model,
because the model has no reliable hook point that runs before every insert
and update path (bulk operations bypass instance hooks), while a check
constraint is enforced by PostgreSQL unconditionally. The belongsTo relation
back to Category uses the same lazy-import pattern as the Category model's
hasMany, for the same circular-import reason.
Customer holds the five scalar columns the README specifies: firstName,
lastName, telephone, email and address. The Tasks checklist for this branch
only lists those scalar fields for Customer, but the Data Model diagram
earlier in the README explicitly draws a 1 to N relation from customers to
orders through orders.customer_id, so a hasMany to Order is declared here
even though it is not spelled out in the Tasks bullet. Leaving it out would
make the diagrammed relation unreachable from the Customer side while the
Order model's belongsTo(Customer) makes it reachable from the other
direction, which would be an inconsistent, half-declared relation.

email has no uniqueness enforcement at the model layer; that constraint
lives in the migration as a UNIQUE column, consistent with how this project
treats the database, not the model, as the source of truth for data
integrity rules that must hold regardless of which code path writes a row.
Order sits at the bottom of the catalog and sales hierarchy, referencing
both Customer and Product through belongsTo relations built the same way as
Product's belongsTo(Category). Its columns are customerId, productId,
quantity and total, matching the README's Column Details table with no
added timestamp fields.

total is stored as a plain column rather than derived on read. The README
describes it as "computed = quantity x unit_price", and that computation
belongs in the orders service that feature/orders will add, evaluated once
at order creation time against the product's unit_price at that moment. If
total were instead computed lazily whenever an order is read, a later price
change on the product would silently rewrite the historical total of every
past order referencing it, which is not what an order record should do.
The User model inherited from the removed --kit=api scaffold composed
UserSchema with withAuthFinder and a DbAccessTokensProvider, none of which
fit the EER_AUTH design this project targets. withAuthFinder pairs with the
access-tokens guard that config/auth.ts already dropped on this branch, and
its accessTokens provider expects an auth_access_tokens table that was
removed along with the scaffold's migrations. Keeping any part of that mixin
around would leave a model that references infrastructure the app no longer
has, so it is replaced outright with a plain BaseModel matching the code
sample in the README's feature/data-modeling section: id, firstName,
lastName, email, password, enabled, accountLocked, and a manyToMany to Role
through the role_user pivot.

Two details carry real weight here. password is declared with
serializeUsing: () => undefined so it can never leak into a JSON response,
API responses included, regardless of which controller or transformer
touches the model later. And the beforeSave hook only re-hashes when
user.$dirty.password is true, which is what makes it safe to save a user
for an unrelated change, such as toggling accountLocked, without silently
hashing an already-hashed password on top of itself and locking the account
out. Password hashing happens nowhere else in the codebase; this hook is
the single point of entry for it.
Role is the hinge of the authorization system: it connects to User through
role_user and to Permission through role_permission. Both pivotTable names
are given explicitly. role_user happens to match Lucid's default pivot
naming convention (singularized table names, alphabetically sorted, joined
with an underscore) for User and Role, but role_permission does not, since
Lucid would otherwise derive permission_role from permissions and roles.
Naming both explicitly keeps the two pivot declarations consistent and
makes the actual table name obvious at the call site instead of depending
on a reader knowing which pairs happen to match the convention.

roleName carries the UNIQUE constraint enforced in the auth schema
migration, since role names like ADMIN and USER are used as stable
identifiers throughout the authorization rules table, not just display
labels, and a duplicate would make role lookups by name ambiguous.
Permission holds resource and action, the two columns the README specifies,
representing a single grantable capability such as (categories, write).

The README only declares the Role side of the role_permission relation, a
manyToMany from Role to Permission. The inverse manyToMany back to Role is
added here so a Permission instance can answer which roles grant it without
requiring a caller to go through Role.query().whereHas() from the other
model. The pivot table already carries both foreign keys with no schema
change needed to support the extra direction, so declaring it costs nothing
and mirrors how Role declares its own two manyToMany relations explicitly.
ActivationToken backs the account activation flow feature/auth will build:
a token is issued for a user, has a creation time, an optional expiry, and
an optional validated timestamp marking when the account was activated.

createdAt uses Lucid's autoCreate so every row gets its timestamp set by
the ORM at insert time, matching the NOT NULL constraint the README places
on this column, while expiresAt and validatedAt stay plain nullable
datetimes since the README does not mark them NOT NULL and their values
depend on business logic that has not run yet at the time the row is first
written (an activation token is not expired or validated the moment it is
created). The belongsTo(User) relation follows the same lazy-import shape
used throughout this branch's models.
BlacklistedToken is what makes JWT revocation possible in a stateless auth
scheme: once a token's jti claim is written here, feature/auth's
AuthMiddleware can reject it on every subsequent authenticated request even
though the JWT itself is still cryptographically valid and unexpired. This
model exists on the data-modeling branch, ahead of the middleware that will
query it, because the table needs to be part of the schema and seedable
infrastructure from the start rather than bolted on later.

jti is unique, matching the README's constraint, since the same token
identifier being blacklisted twice would be redundant at best and could
mask a bug in whatever calls this at worst. token is sized to 768 characters
because a full JWT, header, payload and signature together, comfortably
exceeds the 255-character limit used elsewhere in this schema for shorter
opaque tokens.
PasswordResetToken tracks a single-use token tied to a user with a type and
an expiry date, both NOT NULL per the README's Auth Model table. type is
kept as a plain string rather than a TypeScript union or enum because the
README does not enumerate its possible values, and feature/auth, which owns
the actual reset and activation flows this token type distinguishes
between, is the branch that should decide and document that vocabulary.
Narrowing it here would mean guessing at a contract this branch has no
authority over.

expiryDate is required rather than nullable, unlike the optional expiresAt
on ActivationToken and BlacklistedToken, because a password reset token
without a known expiry would never lapse and could be used indefinitely,
which defeats the point of it being a reset token rather than a permanent
credential.
This migration creates categories, products, customers and orders in that
order, which is not arbitrary: products.category_id references categories,
and orders references both customers and products, so each table must
exist before the one that points at it is created. down() drops them in the
reverse order for the same reason, since PostgreSQL refuses to drop a table
that another table's foreign key still points at.

Two constraints from the README's Column Details table needed more than a
notNullable() call: unit_price and quantity are both documented as "> 0".
Lucid models have no hook that fires uniformly across every write path,
including bulk inserts, so these are enforced as PostgreSQL check
constraints instead, which apply regardless of which code eventually writes
to these tables. total is stored as a plain float column rather than a
generated one; the README describes it as computed from quantity times
unit_price, but that computation has to happen once, in the orders service,
against the product's price at the moment of the order, not be
recalculated by the database every time the row is read.
This migration creates the full authentication and authorization schema in
dependency order: users, roles and permissions first since nothing else
depends on them, then the role_user and role_permission pivot tables that
need two of those three to already exist, then the three token tables that
only depend on users. down() reverses that order so a drop never runs while
a foreign key elsewhere in the schema still references the table being
removed.

A few nullability calls depart from a strict reading of "every column is
required unless the README says otherwise". users.password is left nullable
because the README's own Column Details table is the one column in this
migration that is not marked NOT NULL, which reads as intentional room for
an account to exist before a password is set, such as an invited-but-not-
activated user. The user_id foreign key on all three token tables is
likewise nullable because the README does not mark it NOT NULL there
either, though CASCADE delete still applies to whichever rows do have one,
since a token record has no purpose once its owning account is gone. The
two pivot tables get a composite primary key on their two foreign key
columns, which the README does not ask for explicitly but which is the only
thing stopping the same user-role or role-permission pair from being
inserted twice.
UserSeeder needs to read TEST_ADMIN_EMAIL, TEST_ADMIN_PASSWORD,
TEST_USER_EMAIL and TEST_USER_PASSWORD without touching process.env
directly, per this project's hard rule that all environment access goes
through the typed env service. Those four variables already exist in
.env.test but not in .env.development, so they are declared optional here
rather than required: making them required would mean the app refuses to
boot in development, since Env.create validates against whichever
.env.${NODE_ENV} file is loaded, and .env.development legitimately has no
reason to define test-only credentials.

UserSeeder falls back to literal defaults when these are absent, and those
defaults are the same values already committed to .env.test, so seeding
behaves identically whether it runs against the test database or a
developer's local one.
Seeds five realistic categories that ProductSeeder will attach products to.

updateOrCreateMany is used instead of createMany, keyed on categoryName,
because this project's documented setup command is plain node ace db:seed
with no --files flag, which runs every seeder file under database/seeders
on each invocation rather than only the ones a caller cherry-picks. That
means this file can execute on its own and also get invoked a second time
through MainSeeder in the same db:seed run, so it has to tolerate running
more than once without violating anything or duplicating rows. categoryName
has no database-level uniqueness constraint, but using it as the
idempotency key here is still correct since the seeded category names are
fixed and known in advance.
Seeds eight products distributed across the categories CategorySeeder
creates, looked up by categoryName rather than assumed IDs. Primary keys
are not something a seeder should hardcode, since the same category can
land on a different id depending on what already exists in the database
when the seeder runs; querying by name is the only reliable way to link a
product to the right category regardless of run history.

The requireCategory helper throws if a category is missing rather than
silently skipping the product or inserting a null category_id, which the
schema would reject anyway since category_id is NOT NULL. This turns a
missing dependency into an immediate, readable error at seed time instead
of a confusing foreign key violation from the database. In the documented
node ace db:seed workflow this dependency is satisfied by CategorySeeder
running first, either through alphabetical file ordering or through
MainSeeder's explicit sequencing; the guard exists for the case where
someone runs this seeder in isolation before that has happened.
Seeds four customers spanning a few different locales, since the customers
table is what feature/customers and feature/orders will exercise in their
functional tests. Keyed on email through updateOrCreateMany for the same
idempotency reason as CategorySeeder: db:seed runs every seeder file on
every invocation, so re-running this one must not attempt to insert the
same email twice and hit the UNIQUE constraint the core schema migration
places on customers.email.
Creates the two accounts every subsequent CRUD branch authenticates as in
its functional tests: an ADMIN and a plain USER, with credentials read
through the typed env service rather than hardcoded, and .env.test already
defining the four TEST_* variables this reads. This is the reason the
branch does not have to wait on feature/auth to be merged before other
branches can write authenticated tests against real accounts.

The README's seeder file list has no dedicated RoleSeeder, so the ADMIN and
USER roles are created inline here with updateOrCreateMany before the two
accounts, since role_user assignment needs both roles to already exist and
adding a sixth seeder file the README never mentions felt like the wrong
tradeoff versus a few extra lines in the file that actually needs the
roles. sync([roleId], false) is used for the role assignment specifically
because the false argument tells Lucid not to detach any existing pivot
rows, which is what keeps this idempotent across repeated db:seed runs the
same way the other seeders' updateOrCreateMany calls do.
Runs CategorySeeder, ProductSeeder, CustomerSeeder and UserSeeder in that
explicit order, which is the true dependency order: products need
categories to exist, and users need the roles they create for themselves
before role assignment, but nothing else in this list depends on order.

This class matters less for what node ace db:seed actually does by default
and more as documentation and an explicit entry point. The documented setup
command has no --files flag, so the seeder runner discovers and executes
every file in database/seeders on its own, meaning the four domain seeders
already run independently of this orchestration. Every one of them is
written to be idempotent for exactly that reason, so calling them again
here, in this specific order, never conflicts with their independent runs
elsewhere in the same db:seed invocation. What MainSeeder adds is a single
file that states the intended order in code, and a target for node ace
db:seed --files when someone wants a single deterministic pass instead of
relying on directory listing order.
The pg connection block was written against @adonisjs/core/services/env,
which does not exist in the installed @adonisjs/core version and would
have made every node ace command fail to boot, including migrations
and seeders. Every other config file in this project imports the
typed env instance from the #start/env alias instead, which is where
Env.create actually runs and where the DB_HOST, DB_PORT and related
schema entries are validated.

This fixes a mistake introduced in the earlier commit that switched
the default connection to PostgreSQL, caught while reviewing the
backend agent's report that migrations could not run against the
branch as committed.
This file is produced by node ace migration:run itself, its header
says not to edit it by hand, and the project has committed it as a
snapshot since the first migration was added in feature/core-architecture.
Running the two migrations from this branch against a real PostgreSQL
instance regenerated it automatically, now describing the twelve
tables that make up the core and auth schemas instead of the SQLite
scaffold's users and auth_access_tokens tables.

Verifying migrations against a live database also confirmed the
up and down methods are symmetric: migration:run created all twelve
tables cleanly, migration:rollback --batch=0 reverted both files
without error, and running the migrations again reproduced the same
schema, which is the guarantee the README's migration rollback test
task is checking for.
The Category model declares @hasmany(() => Product) but nothing in the branch had exercised it end to end against the real schema. This spec creates a category with two products and a second unrelated category with its own product, then calls category.load('products') and asserts the loaded relation contains exactly the two products scoped to that category, each with the matching category_id foreign key.

A second test covers the empty case, loading products on a category that has none, so the relation resolves to an empty array instead of throwing or leaving the property undefined. Both behaviors needed direct verification because the categories service in a later branch will rely on preload('products') working correctly for the paginated list endpoint, and a misconfigured foreign key or local key on the relation would otherwise only surface once that endpoint is built.
The orders migration stores total as a plain float written by the service rather than a generated column, precisely so it can freeze the product's unit_price at the moment of purchase instead of letting PostgreSQL recompute it later. That design only holds if the exact value written comes back out unchanged, with no coercion or precision loss between the column mapping and the numeric type in the database.

This spec creates an order with a decimal total computed from quantity times unit_price and checks it on the model instance returned by create, then repeats the check by fetching the same row through a brand new query builder call so the assertion is not just reading back an in-memory value that was never actually persisted. Verifying this now protects the feature/orders branch, which will depend on this same column behaving predictably when it computes and stores totals from real product prices.
The project's hard rule against manual password hashing puts all the weight on the User model's @beforeSave hook: every write path has to end up with a hashed password without the caller remembering to call hash.make() itself. That guarantee has two halves that both needed direct proof, since only testing one would leave the other silently broken. The hook has to hash a brand new plain text password on insert, and it has to hash again when an existing user's password is reassigned and saved.

The third test in this file covers the half that is easy to get wrong: saving a user after changing an unrelated column, like locking the account, must not re-hash an already-hashed password. The hook relies on checking user.$dirty.password before hashing, and without a passing test for the untouched case a future edit could hash the hash on every save, silently corrupting stored passwords and locking every user out at their next login. Each test verifies the real behavior against the hash service's own verify method and, for the untouched case, against a freshly queried row, not just the in-memory instance.
Nothing in the branch had actually exercised the down() side of either migration file against the real test database, only migration:status showing them applied. A schema that only ever migrates up is not proven safe to maintain, since the first time someone actually needs to roll back a bad migration in development is the worst time to discover the down() method was never correct.

This spec drives the MigrationRunner class directly, the same class the migration:run and migration:rollback ace commands call internally, first rolling back the last batch and asserting every table from both the core and auth migration files is actually gone from PostgreSQL's catalog, then running the migrations back up and asserting every table exists again. It deliberately avoids the db.beginGlobalTransaction() pattern used by the model specs in this branch, since DDL statements and the migrator's own bookkeeping do not compose safely with an outer transaction fixture, and it never calls the migrator's close() method, which was found to tear down the shared connection pool that every other unit test file in the process depends on. A catch block re-runs the up direction before rethrowing any assertion failure, so the database is left in the same migrated, runnable state it was found in even when the test itself fails.
Adding the pg type parser to config/database.ts brought in a bare
import of the pg package for its types module, and this project's
installed pg version ships no bundled TypeScript declarations, only
an ESM entry point tsc cannot type on its own. Without a declaration
file, npm run typecheck fails immediately on that import, which is a
required step in both CI workflows.

This is a devDependency only, it has no effect on the runtime build,
it exists purely so tsc can resolve the shape of the pg module during
type checking.
The column option written for hiding the password field does not
exist on this project's installed Lucid version, the correct option
is serializeAs, set to null to drop the field entirely from a
serialized user rather than replacing it with an undefined value.
Running npm run typecheck against the branch as originally committed
failed on this line, since serializeUsing is simply not a member of
the ColumnOptions type here.

The fix mirrors what Lucid's own schema generator writes for the same
column, confirming serializeAs is the version-correct option, and
caught while running the code review's verification pass rather than
trusting the property name from memory.
Every model in this branch types its bigIncrements primary key and
its bigInteger foreign keys as number, following the README's own
User model example, but node-postgres returns BIGINT columns as
strings by default since a bigint can exceed
Number.MAX_SAFE_INTEGER. Verified directly against the live test
database: creating a category and reading its id back gave the
string "1", not the number 1, despite the model declaring id as a
number.

Registering a type parser for OID 20 at the top of config/database.ts,
before any connection opens, converts these values to numbers as soon
as they leave the driver, so every model and every future service
built on top of these ids can rely on the TypeScript type actually
matching the runtime value. This project's row counts stay well under
Number.MAX_SAFE_INTEGER, so parseInt here is a safe and intentional
tradeoff against carrying bigint or string ids through the whole
codebase for values this tutorial will never reach.
This branch is the first to add tests that query real tables, and the
CI Postgres service starts every run from an empty database. Without
a migration step between install and test, npm test has nothing to
query against. Verified this exact failure locally by rolling every
migration back to batch zero, the same starting state as a fresh CI
container, which drops the suite from ten passing tests to two.

The migration step runs with --force so it does not prompt for
confirmation in production mode, matching how the rest of this
workflow already sets NODE_ENV to test through its top level env
block.
Mirrors the same fix applied to ci.yml. pr-checks.yml runs on every
pull request against develop and shares the identical Postgres service
container setup, so it has the same empty-database gap: functional
and unit tests here would fail the moment a spec touches a table this
branch defines, since nothing had applied the migrations first.
The file as originally written did not match this project's Prettier
configuration, npm run lint reported eight formatting errors on it,
which is a required step in both CI workflows and would have failed
every check run on this branch. No logic changed, only line wrapping
and spacing.
Three lines in this file did not match the project's Prettier
configuration, caught by the same npm run lint pass that flagged the
product seeder. No assertions or test behavior changed, only
formatting.
@EdgarEldy
EdgarEldy merged commit 6c3ae23 into develop Aug 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant